-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
feat: add support for read in edge environments #13859
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
🦋 Changeset detectedLatest commit: 3f0328e The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Thanks for this PR! Is there any chance you could continue the work from #11674 so that it's implemented in the Netlify and Vercel adapters too? I think if the reusable
I re-ran the test in CI and it passes. Probably just a flaky test that failed. |
Ah yes. Sorry I missed Rich's PR in my search. Will look into it. Thanks for the input! |
dc18769
to
6da55ed
Compare
@eltigerchino extracted into a
Also, I haven't added I can spawn up some projects in netlify & vercel and do some manual tests, but I feel like i shouldn't block progress here. How about I do that in a separate PR? Thanks |
…t util for server read
Thank you!
Yeah, manual testing for those are fine. There is no local environment for Vercel and like you said, Netlify's edge doesn't work locally. |
Can this be prioritized? It's very cumbersome to use static assets with Cloudflare with unexpected outcome. Vercel and other can be added later |
I've added support to the Netlify and Vercel adapters. Also added a test suite for Netlify now that the local dev environment supports edge functions. |
Thanks @eltigerchino. Apologies I couldn't find time to follow through with Netlify & Vercel. Appreciate you taking on the responsibility! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@eltigerchino I found a solution that avoids adding a new export: tweak SvelteKit's read
implementation to handle async values. Could you take another look?
(all SvelteKit tests fail currently but it was the only solution to make the adapter tests succed with the checks; annoying interim failures, which is why we should merge and release this in one go)
] | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does Netlify require this or why do we have a deno.lock here? (same for the other test)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a build artifact from running the app with the Netlify CLI. I think it does another bundle at that point. I'll add it to to the .gitignore
Thanks! That's a really nice solution. |
closes #12446
closes #11674
This PR helps with the cloudflare part of #12446. Added a test case and I also tested in a project bootstrapped with
create cloudflare@latest
. Hope i didn't miss anything 🤞.Thanks all.
Please don't delete this checklist! Before submitting the PR, please make sure you do the following:
Tests
pnpm test
and lint the project withpnpm lint
andpnpm check
Changesets
pnpm changeset
and following the prompts. Changesets that add features should beminor
and those that fix bugs should bepatch
. Please prefix changeset messages withfeat:
,fix:
, orchore:
.Edits